Skip to content

bucket-A3: add core/coin_params.hpp scaffolding (SAFE-ADDITIVE) - #49

Merged
frstrtr merged 1 commit into
masterfrom
a3/core-coin-params
May 31, 2026
Merged

bucket-A3: add core/coin_params.hpp scaffolding (SAFE-ADDITIVE)#49
frstrtr merged 1 commit into
masterfrom
a3/core-coin-params

Conversation

@frstrtr

@frstrtr frstrtr commented May 30, 2026

Copy link
Copy Markdown
Owner

Summary

Adds the coin-agnostic core/coin_params.hpp scaffolding header — the shared description of a coin's consensus/network constants that per-coin layers populate and core consumes. Purely additive: one new file, nothing on master #includes it yet (audit classification: SAFE-ADDITIVE).

1 file, +133:

  • new — src/core/coin_params.hpp

Cherry-picked from dash-spv-embedded HEAD 8249dc28.

Context

Part 3 of the bucket-A sub-PRs for the multi-coin landing strategy. Follows #46 (A1, bitcoin_family base) and #48 (A2, enhanced_node template).

Note: the strategy doc originally scoped A3 as 2 files (coin_params.hpp + core/pow.hpp). pow.hpp was absorbed into A1 as a hard transitive dependency of bitcoin_family/coin/chain_params.hpp, so A3 shrinks to the single coin_params.hpp.

Why this is safe for LTC+DOGE

  • Brand-new file. git grep confirms nothing on master #includes coin_params.hpp, so it is never pulled into a translation unit on this branch — zero impact on the LTC/DOGE/legacy build. The plumbing that actually consumes CoinParams (LTC + c2pool_refactored.cpp) is bucket A7, reviewed separately.

Merge ordering (read before merging)

coin_params.hpp #includes core/pow.hpp, which ships in A1 (#46). This is a merge-ordering dependency, not a code dependency on this branch:

  • Since nothing #includes coin_params.hpp on master, it is not compiled here and CI is green without pow.hpp present.
  • The include resolves the moment A1 merges. Land order is A1 → A2/A3/A4 back-to-back, per the plan, so this is satisfied naturally.
  • pow.hpp is intentionally not re-added in this PR — it already ships in A1; duplicating it would create a duplicate-add merge conflict.

CI expectation

  • Legacy linux: + cross-platform + CodeQL: green (file uncompiled).
  • coin-matrix entries: skip fast on this branch.

Reviewer

Agent-side reviewer: ltc-doge-production-steward (watches the branch). Integrator (cc) routes human review.

Test plan

  • Legacy linux: CI green
  • CodeQL + cross-platform green
  • No file other than src/core/coin_params.hpp touched (directory-contract spot-check)

@frstrtr
bucket-A3: add core/coin_params.hpp scaffolding (SAFE-ADDITIVE)
84ad5d4 
New coin-agnostic CoinParams scaffolding header — the shared description of a
coin's consensus/network constants that the per-coin layers will populate and
the core will consume. Purely additive: one new file, nothing on master
includes it yet (the LTC/c2pool plumbing that consumes it is bucket A7), so no
existing translation unit compiles it and LTC+DOGE behavior is unchanged.

coin_params.hpp #includes core/pow.hpp, which lands in bucket A1 (#46) — that
gives A3 a merge-ordering dependency on A1, not a code dependency on this
branch: since nothing #includes coin_params.hpp on master, it is never pulled
into a TU here and CI stays green. The include resolves once A1 is merged
(A1 → A2/A3/A4 back-to-back per the landing plan).

pow.hpp is intentionally NOT re-added here — it already ships in A1; adding it
again would create a duplicate-add merge conflict.

Cherry-picked from dash-spv-embedded HEAD 8249dc2.

1 file, +133:
- new  src/core/coin_params.hpp
This was referenced May 30, 2026
Merged
Merged
@frstrtr

frstrtr commented May 31, 2026

Copy link
Copy Markdown
Owner Author

A3 review verdict — ltc-doge-production-steward (LTC+DOGE production perspective)

APPROVE-ready. Reviewed at A1 depth. All five integrator verification points pass.

Diff scope

1 file, +133, additive only: src/core/coin_params.hpp (new). No other path touched — directory-contract spot-check passes.

1. Include resolution on current master (cc2fd4e)

  • #include "pow.hpp" → src/core/pow.hpp present on master, resolves.
  • #include "uint256.hpp" → src/core/uint256.hpp present, resolves.
  • All function-type aliases used by CoinParams are defined in pow.hpp: PowFunc, BlockHashFunc, SubsidyFunc, DonationScriptFunc. uint256 (for max_target) from uint256.hpp. No undefined symbol.
  • pow.hpp is not re-added here (diff = single file), so no duplicate-add merge conflict — consistent with the PR body. The merge-ordering note (A1 → A2/A3/A4) is correct; pow.hpp already shipped in A1, so the include is satisfied the moment this lands on a master that has A1.

2. SAFE-ADDITIVE confirmed

git grep coin_params.hpp on master returns nothing — the header is never pulled into a translation unit on this branch, so zero impact on the LTC/DOGE/legacy build. Green standalone CI is the correct gate, and it is green (Linux x86_64, Windows, macOS arm64, CodeQL, all coin smokes pass).

3. No hidden dependency

Only two deps (pow.hpp, uint256.hpp), both on master. No third include that would surface only when A4/A7 consumes CoinParams. Self-contained.

4. ODR / multiple-definition (downstream A4+ safety)

  • #pragma once guard present.
  • The two convenience accessors (active_identifier_hex, active_prefix_hex) are defined in-class → implicitly inline → safe to include in multiple TUs.
  • No free-function definitions, no namespace-scope variable definitions, no static data members, no templates. Nothing that can cause a multiple-definition link error when A4/A7 includes this from several TUs.

LTC+DOGE notes (non-blocking)

  • Vardiff defaults documented as matching p2pool-merged-v36 LTC (N−1 denominator, use_full_window=false) and the Dash three-trigger override path — semantics look right per coin; the actual factory population is A7, reviewed there.
  • current_share_version comment (36 LTC / 16 Dash) matches our canary expectations.

@Integrator — ready to approve. No changes requested. Merge ordering A1 → A2/A3/A4 back-to-back is the only constraint, and A1 is already on master. Formal GH approval is still gated by the workstation-gh-is-PR-author self-review block (same as #46), so this comment is the binding verdict on my side.

frstrtr added a commit that referenced this pull request May 31, 2026
@frstrtr
fix(parse): drop non-finite amounts in parseLegacyShape
837ae89 
A raw Infinity in the legacy flat-map shape (value is a bare number)
was assigned to amount without finite filtering. Infinity > 0 is true,
so the row survived and poisoned totalPrimary, producing a non-finite
snapshot total. Route the value through num(), which clamps non-finite
values to 0 so the existing !(amount > 0) guard discards the row.

Surfaced by the fast-check property parseSnapshot: output always has
required keys with correct types (seed -1679627146, counterexample
[Infinity]) flaking the Web-static verify gate on PRs #49/#50.
@frstrtr frstrtr mentioned this pull request May 31, 2026
Merged
@frstrtr
frstrtr merged commit 9063353 into master May 31, 2026
18 of 19 checks passed
@frstrtr frstrtr mentioned this pull request Jun 19, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@frstrtr